ign -> gz Migrate Internal CMake Vars : gz-cmake #275

methylDragon · 2022-07-09T21:50:32Z

Built on top of: #274

In an effort to keep track of what has already been tick-tocked, I placed the Deprecated. comment (in most cases) on the same line so it appears on grep.

Try it: grep -ri set\(ign

BREAKING NOTES

IGNITION_DOXYGEN_TAGFILES is set by DOWNSTREAM libraries and used in this UPSTREAM library... I am just going to hard-tock it and remember to update the downstream usage since it's just for docs...
- This won't lead to a compile error, but will lead to malformed docs until the downstream libraries are tocked.

The next few ones look like they're breaking (because they change the install path to gz), but local builds work.
Release repos might need to be updated to have the topline ignition -> gz.

Change default include directory from include/ignition to include/gz (I think in GzConfigureBuild.cmake and GzPackaging.cmake?)
PROJECT_INCLUDE_DIR stores ignition/${IGN_DESIGNATION} -> gz/...
- This would be breaking, but all downstream uses actually bypass this block, so I think it's safe to adjust, I think
- This crucially changes the install directory to gz instead of ignition, likely necessitating release repository changes, but no other issues if packages are rebuilt accordingly

See: e1c4a35

Pending??

What should be done about version_info.json.in (I've provisionally migrated ignition to gz here)

Hard-Tocks

Usually because they're internal or aren't used anywhere else in the entire Gz stack

GitHub workflow targets
Erm.. docs only mentions?
- IGN_SANITIZERS
- USE_IGN_RECOMMENDED_FLAGS
GzCodeCoverage.cmake:
- IGN_SETUP_TARGET_FOR_COVERAGE
Top level gz-cmake CMakeLists.txt
- ign_pkgconfig_xxx
- ign_utilities_xxx
- IGN_PC_CONFIG_RELATIVE_PATH_TO_PREFIX
GzPackaging.cmake:
- IGNITION_CMAKE_DIR
- IGNITION_CMAKE_VERSION_MAJOR
GzCreateDocs.cmake
- IGNITION_DOXYGEN_GENTAGFILE
- IGNITION_DOXYGEN_INPUT
- IGNITION_DOXYGEN_IMAGE_PATH
- IGNITION_DOXYGEN_API_MAINPAGE_MD
- IGNITION_DOXYGEN_GENHTML
- IGNITION_DOXYGEN_AUTOGENERATED_DOC
- IGNITION_DOXYGEN_TUTORIALS_MAINPAGE_MD
- IGNITION_DOXYGEN_TUTORIALS_DIR
- IGNITION_DOXYGEN_ADDITIONAL_INPUT_DIRS
GzRonn2Man.cmake
- ign_add_manpage_target
gz-config.cmake.in
- set_and_check(@PKG_NAME@_INCLUDE_DIRS "@PACKAGE_IGN_INCLUDE_INSTALL_DIR_FULL@") this is the only occurance of that IGN variable. I don't know why

Tick-Tocks

Every variable that is set with an IGNITION/IGN prefix, generally in .cmake files which are meant to be included (internal references hard-tocked, but the set call itself is ticktocked.)
gz-cmake-config.cmake.in:
- IGN_INCLUDE_INSTALL_DIR_FULL
- IGNITION_CMAKE_VERSION_MAJOR
- IGNITION_CMAKE_DOXYGEN_DIR (Also in top level CMakeLists.txt... it's confusing, I know)
- IGNITION_CMAKE_CODECHECK_DIR
- IGNITION_CMAKE_BENCHMARK_DIR
- IGNITION_CMAKE_TOOLS_DIR
All these IGN_DESIGNATION variables...
- IGN_DESIGNATION
- IGN_DESIGNATION_LOWER
- IGN_DESIGNATION_UPPER
- IGN_DESIGNATION_FIRST_LETTER
- IGN_DESIGNATION_CAP
GzPackaging.cmake:
- IGN_INCLUDE_INSTALL_DIR
- IGN_INCLUDE_INSTALL_DIR_POSTFIX
- IGN_INCLUDE_INSTALL_DIR_FULL
- IGN_DATA_INSTALL_DIR_POSTFIX
- IGN_DATA_INSTALL_DIR
- IGN_LIB_INSTALL_DIR
- IGN_BIN_INSTALL_DIR
GzConfigureBuild.cmake
- IGN_CXX_XXX
- IGN_KNOWN_CXX_STANDARDS
- CMakeLists.txt search path now prioritizes gz over ignition
GzSetCompilerFlags.cmake
- IGN_ADD_fPIC_TO_LIBRARIES
- IGN_USE_STATIC_RUNTIME (option and cache variable)
GzPython.cmake
- IGN_PYTHON_VERSION
Hmhm
- REPLACE_IGNITION_INCLUDE_PATH
- NO_IGNITION_PREFIX

cmake/GzConfigureProject.cmake

Signed-off-by: methylDragon <[email protected]>

methylDragon · 2022-07-12T03:26:23Z

This PR can be squashed

Signed-off-by: methylDragon <[email protected]>

chapulina

Partial review so far

Migration.md

tools/doc_check.sh

cmake/GzSanitizers.cmake

…uide Signed-off-by: methylDragon <[email protected]>

chapulina

Finished doing a pass on the code, still need to test locally with other libraries

chapulina · 2022-07-12T17:29:52Z

cmake/GzPython.cmake

@@ -43,3 +43,6 @@ endif()
 if(Python3_EXECUTABLE AND NOT PYTHON_EXECUTABLE)
  set(PYTHON_EXECUTABLE ${Python3_EXECUTABLE})
 endif()
+
+set(IGN_PYTHON_VERSION ${GZ_PYTHON_VERSION} CACHE STRING  # TODO(CH3): Deprecated. Remove on tock.
+  "Deprecated. Use [GZ_PYTHON_VERSION] instead! Specify specific Python version to use ('major.minor' or 'major')")


Should this be up top?

cmake/GzPython.cmake

chapulina · 2022-07-12T17:37:10Z

cmake/GzSetCompilerFlags.cmake

  # part of a plugin-based framework.
-  option(IGN_USE_STATIC_RUNTIME "Use the static runtime (strongly discouraged)" OFF)
+
+  if(IGN_USE_STATIC_RUNTIME)  # TODO(CH3): Deprecated. Remove on tock.


Do we still need the option() line before checking if it's set?

Option will create a cache variable which can then be set via CLI, if I'm not wrong, so that's why I think having the option there is necessary

chapulina · 2022-07-12T17:38:07Z

cmake/GzSetCompilerFlags.cmake

  if(BUILD_SHARED_LIBS)
    # Users should not choose the static runtime unless they are compiling a
    # static library, so we completely disable this option if BUILD_SHARED_LIBS
    # is turned on.
-    set(IGN_USE_STATIC_RUNTIME OFF CACHE BOOL "Use the static runtime (strongly discouraged)" FORCE)
+    set(GZ_USE_STATIC_RUNTIME OFF CACHE BOOL "Use the static runtime (strongly discouraged)" FORCE)
+    set(IGN_USE_STATIC_RUNTIME ${GZ_USE_STATIC_RUNTIME}  # TODO(CH3): Deprecated. Remove on tock.


If I understand correctly, this is internally setting the variable. So we should only need to set GZ_ and use that.

Signed-off-by: methylDragon <[email protected]>

github-actions bot added the 🌱 garden Ignition Garden label Jul 9, 2022

methylDragon mentioned this pull request Jul 9, 2022

Rename Ignition to Gazebo gazebo-tooling/release-tools#698

Open

69 tasks

methylDragon force-pushed the migrate_cmake_vars branch 7 times, most recently from e1c4a35 to a1065a1 Compare July 11, 2022 07:44

methylDragon marked this pull request as ready for review July 11, 2022 07:46

methylDragon requested review from mxgrey and j-rivero as code owners July 11, 2022 07:46

chapulina added the ign to gz Renaming Ignition to Gazebo. label Jul 11, 2022

chapulina self-requested a review July 11, 2022 15:14

methylDragon force-pushed the migrate_cmake_vars branch from fb2f2f0 to e7974ff Compare July 11, 2022 22:32

methylDragon mentioned this pull request Jul 12, 2022

ign to gz : Remaining Files, and Python Libraries/Namespaces gazebo-tooling/release-tools#765

Closed

32 tasks

scpeters reviewed Jul 12, 2022

View reviewed changes

cmake/GzConfigureProject.cmake Outdated Show resolved Hide resolved

cmake/GzConfigureProject.cmake Outdated Show resolved Hide resolved

scpeters mentioned this pull request Jul 12, 2022

ign -> gz CMake, Python, Partial Source, and File Migrations : gz-math gazebosim/gz-math#460

Closed

methylDragon added 6 commits July 11, 2022 20:22

Migrate CMake vars and options

d9ac05a

Signed-off-by: methylDragon <[email protected]>

Potentially breaking: Migrate ignition include directory

7f9dd15

Signed-off-by: methylDragon <[email protected]>

Update JSON

c0de72f

Signed-off-by: methylDragon <[email protected]>

Add migration line

926c7bf

Signed-off-by: methylDragon <[email protected]>

Update JSON to have gz and ignition side by side

652fa8f

Signed-off-by: methylDragon <[email protected]>

Change option logic

efe92a3

Signed-off-by: methylDragon <[email protected]>

methylDragon force-pushed the migrate_cmake_vars branch from e7974ff to efe92a3 Compare July 12, 2022 03:23

Use REPLACE_INCLUDE_PATH and NO_PROJECT_PREFIX

fb9a7eb

Signed-off-by: methylDragon <[email protected]>

methylDragon force-pushed the migrate_cmake_vars branch from 141d2ae to fb9a7eb Compare July 12, 2022 07:38

chapulina suggested changes Jul 12, 2022

View reviewed changes

Migration.md Outdated Show resolved Hide resolved

tools/doc_check.sh Show resolved Hide resolved

cmake/GzSanitizers.cmake Show resolved Hide resolved

Apply fixes to filenames, ticktock IGN_SANITIZER, and fix migration g…

84e0509

…uide Signed-off-by: methylDragon <[email protected]>

chapulina suggested changes Jul 12, 2022

View reviewed changes

Update variable tick-tocks

5f4f308

Signed-off-by: methylDragon <[email protected]>

methylDragon closed this Jul 12, 2022

methylDragon deleted the migrate_cmake_vars branch July 12, 2022 18:28

methylDragon mentioned this pull request Jul 12, 2022

ign -> gz Migrate Internal CMake Vars : gz-cmake #277

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ign -> gz Migrate Internal CMake Vars : gz-cmake #275

ign -> gz Migrate Internal CMake Vars : gz-cmake #275

methylDragon commented Jul 9, 2022 •

edited

Loading

methylDragon commented Jul 12, 2022

chapulina left a comment

chapulina left a comment

chapulina Jul 12, 2022

methylDragon Jul 12, 2022

chapulina Jul 12, 2022

methylDragon Jul 12, 2022 •

edited

Loading

chapulina Jul 12, 2022

methylDragon Jul 12, 2022

ign -> gz Migrate Internal CMake Vars : gz-cmake #275

ign -> gz Migrate Internal CMake Vars : gz-cmake #275

Conversation

methylDragon commented Jul 9, 2022 • edited Loading

BREAKING NOTES

Pending??

Hard-Tocks

Tick-Tocks

methylDragon commented Jul 12, 2022

chapulina left a comment

Choose a reason for hiding this comment

chapulina left a comment

Choose a reason for hiding this comment

chapulina Jul 12, 2022

Choose a reason for hiding this comment

methylDragon Jul 12, 2022

Choose a reason for hiding this comment

chapulina Jul 12, 2022

Choose a reason for hiding this comment

methylDragon Jul 12, 2022 • edited Loading

Choose a reason for hiding this comment

chapulina Jul 12, 2022

Choose a reason for hiding this comment

methylDragon Jul 12, 2022

Choose a reason for hiding this comment

methylDragon commented Jul 9, 2022 •

edited

Loading

methylDragon Jul 12, 2022 •

edited

Loading